Don't edit current when changing docks (reverted) #90816
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Looks like it's needed when making a dock floating and back. RABTXSEJw3.mp4But it's only needed for the dock being moved and only for some docks, so calling a method that affects editor state globally is too much 🤔 EDIT: |
kitbdev
force-pushed
the
dock-dont-edit-current
branch
from
c8b93a3
to
8fd98bb
Compare
|
Changed Editor Inspector to only clean up on predelete and not in exit tree. I think the Inspector was the only dock had this problem. Moving the Inspector should now keep its state. |
kitbdev
force-pushed
the
dock-dont-edit-current
branch
from
8fd98bb
to
55711b2
Compare
This comment was marked as resolved.
This comment was marked as resolved.
KoBeWi
approved these changes
Apr 19, 2024
|
I noticed the error a little late and made a second push after my comment. |
|
Thanks! |
|
Heads up that this commit is causing a crash on quit – I suspect due to a use-after-free. Important The issue appears to occur when the Visual Profiler under the Debugger tab is open, when closing Godot.
If I revert this specific change, the crash goes away: diff --git a/editor/editor_inspector.cpp b/editor/editor_inspector.cpp
--- a/editor/editor_inspector.cpp (revision bde404fb39494284cbdfa641b8e4759e7ee2ce42)
+++ b/editor/editor_inspector.cpp (date 1713825339106)
@@ -4028,6 +4028,7 @@
if (!sub_inspector) {
get_tree()->disconnect("node_removed", callable_mp(this, &EditorInspector::_node_removed));
}
+ edit(nullptr);
} break;
case NOTIFICATION_VISIBILITY_CHANGED: { |
|
Backtrace (which I've gotten on every exit of the editor so far): |
|
@Calinou looks very similar to mine on macOS 👍🏻 |
|
If I close the Debugger tab before exiting, it doesn't crash |
|
I cannot reproduce the crash on Windows. |
|
I can reproduce the crash on Linux and macOS (and can confirm that re-added removed
|
|
For now I reverted the PR to fix the crash. This can be redone with a fix for the regression (and we'll make sure to test on Linux and macOS). |
akien-mga
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes Toggling distraction-free mode selects (or opens) a fixed script in the script list #90654
Before Refactor and fix issues in Editor Dock Manager #88003,
edit_current was called in move dock left/right, make/close window, move to/from bottom, and dock select click.
But not in drag and drop, changing dock tab, distraction free, load layout, or feature profiles.
In that PR I unified them to all call edit_current, but I don't think they need to call it at all. If any of them do, let me know.